r? @petrochenkov

rustbot has assigned @petrochenkov.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@bors try

☀️ Try build successful (CI)
Build commit: 0de49ae (0de49ae9e81f9a1e7df6f0783824ce94ed18e8a9, parent: a2db9280539229a3b8a084a09886670a57bc7e9c)

@craterbot check

👌 Experiment pr-147185 created and queued.
🤖 Automatically detected try build 0de49ae
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

🚧 Experiment pr-147185 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

🎉 Experiment pr-147185 is completed!
📊 8 regressed and 3 fixed (708741 total)
📊 1547 spurious results on the retry-regessed-list.txt, consider a retry¹ if this is a significant amount.
📰 Open the summary report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

This found

• a few cases of abi_stable-0.9.3, an outdated crate version where the lint probably has been fixed in more recent versions (and I highly doubt there's any repr(C) here, this is going to be about the non-exhaustive part of the lint)
• dynamic_graph-0.1.5, a crate that didn't have any updates in 4 years, here, involving a use of generativity::Id. I don't actually understand why this happens as the Id type is repr(transparent) itself... but nothing here is repr(C) so this is also almost certainly about the non-exhaustive part of the lint, not the repr(C) part.

EDIT: It later got confirmed that these crates indeed already trigger the lint before this PR.

@bors try

☀️ Try build successful (CI)
Build commit: e19b5cc (e19b5cc0f024358aaaecf2776a5291fa5a95b623, parent: e2c96cc06bdbdbc6f59c7551194d6a742260d6ff)

@craterbot check crates=https://crater-reports.s3.amazonaws.com/pr-147185/retry-regressed-list.txt p=1

👌 Experiment pr-147185-1 created and queued.
🤖 Automatically detected try build e19b5cc
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

🚧 Experiment pr-147185-1 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

🎉 Experiment pr-147185-1 is completed!
📊 6 regressed and 0 fixed (1534 total)
📊 90 spurious results on the retry-regessed-list.txt, consider a retry¹ if this is a significant amount.
📰 Open the summary report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@rfcbot fcp merge

We talked about this in the lang meeting today. We agreed on these answers to #147185 (comment):

Should we phase out repr(transparent) ignoring repr(C) 1-ZST fields?

Yes, we should phase this out.

• If yes, should we do this as part of the existing repr_transparent_external_private_fields lint (under some new name)? If it should be a new separate lint, please specify how exactly the two lints should behave. Specifically, which lints should fire for a repr(transparent) type that has a 1-ZST non-exhaustive field, a 1-ZST repr(C) field, and a non-ZST field? That type would only get rejected once we make both the non-exhaustive and the repr(C) restriction into hard errors, but we have no good way to fire a lint only if both lints are enabled, and the logic to disentangle these cases would become quite messy. It just doesn't seem worth it for something that so few people will even see.

We are fine with combining them. This is a pretty rare case so it doesn't seem worth splitting them.

• What should the new lint (combined or not) be called?

The name we eventually settled on in the meeting was repr_transparent_non_zst_fields.

• Should the lint fire in dependencies immediately? Even for the combined lint, crater found only two rarely-used old crates that would trigger it.

Given that this is so rare, we agreed to move immediately to deny-by-default, warn in deps.

@rfcbot fcp merge

Team member @tmandry has proposed to merge this. The next step is review by the rest of the tagged team members:

• @joshtriplett
• @nikomatsakis
• @scottmcm
• @tmandry
• @traviscross

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

@rfcbot reviewed

The motivation is, e.g.:

If we go with rust-lang/rfcs#3845 (or another approach for fixing repr(C)), they will anyway not be ZST on all targets any more

I.e., these repr(C) types are (maybe) not guaranteed to be 1-ZSTs always everywhere. That's what we were trying to suggest in the name. We also had earlier considered repr_transparent_bad_fields.

Given that rust-lang/rfcs#3845 hasn't been accepted yet, what should the lint say to explain that a repr(C) type is not a ZST? Something about future compiler versions?

Right. If we think we might change it in the future, that means we're not actually guaranteeing it today, as a language matter, from the perspective of our stability guarantees. So, e.g., maybe "this type may not be zero-sized on all targets in the future" or even "this type is not guaranteed to be zero-sized on all targets". I.e., maybe it happens to be true today, but in adding this lint, we're disclaiming this guarantee.

🔔 This is now entering its final comment period, as per the review above. 🔔